fix(providers): drop removed MultiEdit tool from Claude deny-rule list#8
Closed
fablerlabs wants to merge 214 commits into
Closed
fix(providers): drop removed MultiEdit tool from Claude deny-rule list#8fablerlabs wants to merge 214 commits into
fablerlabs wants to merge 214 commits into
Conversation
- consolidate 11 files down to clear ownership: AGENTS.md is canonical agent guide, CLAUDE.md points to it as must-read - fix schema: add blocked status, require size/risk_level/planning_log/ created/updated, add move_to field, require description on criteria - differentiate strict vs standard validation profiles (boundary_check) - add resume protocol to exec.md - replace heavy ReAct logging with lightweight per-phase summaries - add complete example spec in .ai/specs/examples/ - slim .ai/README.md and specs/README.md to remove duplication - rewrite README.md as clean project overview
Add all missing fields: touchpoint owners/links, constraints.approvals_required, info_sources, planning_log notes, acceptance_criteria validation/result.output/ result.notes, definition_of_done notes, deviations array, integration and custom criterion types, manual validation type, and documentation criterion.
- Python CLI (cli/trellis) with init, new, list, status, validate, approve, start, complete, fail, cancel commands - Submodule-first workspace: trellis init creates symlinks, copies templates, installs CLI to ~/.local/bin - Update docs for CLI usage and workspace architecture
- trellis exec: runs acceptance criteria commands, records pass/fail results back into the spec YAML - trellis audit: compares declared spec files against git diff to detect scope creep - trellis diff: shows git history and uncommitted changes for a spec - auto planning_log entries on all status transitions - self-eval honesty checks on complete (warns on rubber-stamp 10/10) - warn if completing without exec results - trim unused schema fields (constraints, info_sources) - add result/executed_at/result_output to acceptance_criteria schema
- config overlay: .ai/config.local.yaml merges on top of base config so submodule stays updatable without losing project customizations - trellis report: aggregate stats across all specs (status breakdown, monthly activity, self-eval distribution, exec pass rates, phase stats) - replace fragile regex YAML parsing with indent-aware parser that handles both acceptance_criteria and validation blocks, dod_id fields, escaped quotes in commands, and varying indentation levels - trellis init now creates starter config.local.yaml - bump version to 1.1.0
- trellis repo: ignore __pycache__, .DS_Store, .ai/logs/ - trellis init: appends Trellis entries to project .gitignore (logs dir ignored, specs committed for audit trail)
Replace the symlink-from-submodule init with direct file copying. trellis init now copies config, schemas, prompts, and templates directly into the project. Added install.sh for one-liner setup. Bumped to v1.2.0.
Acceptance criteria can now declare a `cwd` field (relative to workspace
root) so commands run in the correct submodule directory. Validates that
the path exists and doesn't escape the workspace root. Commands without
cwd still run from root (backward-compatible).
acceptance_criteria:
- id: ac1
cwd: api
command: "bundle exec rspec spec/..."
Updated spec schema and README.
approve now runs schema validation before promoting a spec. Invalid specs are rejected with actionable error messages. exec --resume (-r) skips criteria that already have result: "pass" recorded in the spec. Useful for iterative development — fix a failing criterion, re-run with -r, and only the pending/failed ones execute. Extracted validate_spec() helper so validation logic is reusable. Bumped to v1.3.0.
exec now parses the expected field against command output instead of only checking exit codes. Handles "0 failures" (catches N>0 in output), "no matches", "exit code N", "Syntax OK", and literal substring match. approve now rejects specs containing TODO placeholders in actionable fields (command, content_spec, description, file, list items). Specs can set task.context.cwd as a default working directory for all acceptance criteria. Individual criteria can still override. Reduces repetition in monorepo specs where all commands target the same submodule. Bumped to v1.3.0.
- New `trellis review` command: runs automated passes (spec_compliance, scope_drift), scaffolds review file with context, prints adversarial prompt to stdout for the agent to execute - Review file accumulates rounds — each `trellis review` appends a new numbered section. Prior rounds provide context for subsequent reviewers. - `trellis complete` now gates on review: reads latest review section, parses verdict, refuses to archive on fail. --force to override. - Review verdict (with review_rounds count) recorded in archived spec - New review.md prompt template with three attack vectors: regression hunt, convention violations, defect scan - Updated spec schema, config, exec.md, AGENTS.md, README.md
- CLAUDE.md: add review mode, spec management rule - AGENTS.md: add review mode, spec management rule, review.md in key paths - .ai/OPERATORS.md: add trellis review to CLI, lifecycle, verification workflow - .ai/README.md: add review step, review.md and reviews/ to directory structure - .ai/specs/README.md: add review step to workflow, review field to anatomy
Enforce structured review artifacts and the audited human-reviewed override path. Add the review gate smoke workflow and README badge, make the smoke harness portable in CI, and ignore local review artifacts.
The review scaffold now includes Regression Hunt, Convention Check, and Defect Scan headings. parse_review_file tracks which are empty, and cmd_complete gates on all three having content. This prevents agents from rubber-stamping reviews by skipping adversarial vectors.
Make the review pipeline config-driven with ordered built-in passes and Review Artifact v3.\n\nAlso harden acceptance execution with nested result resume handling, per-criterion timeout support, generic pass expectations, refreshed docs, and expanded smoke coverage.
Use grep-based ordering checks in the smoke harness when ripgrep is unavailable so the GitHub Actions runner matches local behavior.
Handle nested acceptance result blocks and self_eval totals in complete/report. Also count phase statuses from the phases block only and cover the regressions with smoke tests.
update archived spec yamls to reference scafld; minor readme/cli/test updates
- new `scafld harden <task-id>` command scaffolds HARDEN MODE prompt and appends a round to `harden_rounds`; `--mark-passed` closes it - optional schema fields `harden_status` and `harden_rounds` (both non-required, backwards-compatible for specs missing them) - anti-hallucination grounding contract: every question must cite `spec_gap:<field>`, `code:<file>:<line>`, or `archive:<task_id>` - `scafld approve` intentionally unchanged - harden is operator-driven and never gates approval - spec template emits `harden_status: "not_run"` for discoverability - `scafld report` surfaces harden adoption (no gating, non-punitive) - docs, AGENTS, OPERATORS, planning, lifecycle, and spec-schema updated to describe the phase as optional - smoke test + CI wire-up enforces the optional-by-design contract
Under CI=true the test hit verify's CI-policy branch instead of the non-CI receipt-path resolution it asserts (red since the finalize cutover). Make it hermetic.
Keep the deterministic-protocol tagline; the finish line is now finalize signing an ed25519 receipt that scafld verify re-checks in CI, not a self-graded complete.
…ipts A failed envelope encode previously exited 0 with corrupt stdout; encode errors now print on the stream and force a non-success exit. Canonical receipt serialization checks marshal errors instead of discarding them.
Receipt files write atomically; latest.json is written only after the receipt is anchored in the session ledger. Ledger appends take a flock on unix so concurrent scafld processes cannot drop each other's entries. The signer refuses group- or world-accessible private key files.
fileDigests spawned one git cat-file subprocess per tree file per snapshot and hashed CombinedOutput, letting stderr noise reach the digest. One --batch process now streams deduped blobs over a dedicated stdout pipe.
The mutation guard recomputes only the tree fingerprint through a new Snapshotter.TreeSHA port method instead of a second full snapshot. The reviewer evidence reuses the per-file facts the signed snapshot already hashed, and reads evidence bytes through one cat-file --batch process instead of one subprocess per file.
provider.go keeps selection and receipt-grade wiring; provider client implementations move to clients.go and shared invocation, dossier, and event helpers to invoke.go. The duplicated review/harden progressLabel and first helpers collapse into the providerinfo package.
Keep completion ledger-authoritative after blocking reviews: a later passing review may clear a failure only when build evidence or changed review-attempt fingerprints prove a repair happened. Also make status avoid stale projected complete commands and fail finalize before invoking a provider on empty review evidence.
Describe targeted validation as the standard build-gate default and reserve full-suite evidence for release gates, shared framework changes, or explicit operator requests.
The claude provider hardcodes --disallowedTools "Agent,Task,Bash,Edit,MultiEdit, Write,NotebookEdit". MultiEdit no longer exists in Claude Code, and the CLI hard-fails on an unknown deny-rule name: Permission deny rule "MultiEdit" matches no known tool - check for typos. So the provider process exits before it can call submit_review, and every `scafld review --provider claude` run dies with: review_failed: provider produced no submission Reproduced against a current Claude Code CLI (exit 4). Removing MultiEdit from the list makes the same invocation succeed; write access is still denied by Edit, Write and NotebookEdit, so the sandbox posture is unchanged. provider_test.go and evidence_sandbox_test.go assert the literal deny-rule string, so they pinned the broken value and stayed green; both are updated.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
scafld review --provider claudecannot run at all against a current Claude Code CLI.internal/adapters/providers/clients.go:413hardcodes the deny list:MultiEdithas been removed from Claude Code, and the CLI treats an unknown deny-rule name as a fatal error rather than ignoring it:The provider process therefore exits before it can ever call
submit_review, and the review gate fails closed:Reproduced live on v2.5.0 (exit 4). Because the gate fails closed, this is not a silent-pass bug — but it does make the flagship Claude review provider unusable.
Why the tests didn't catch it
provider_test.go:872andevidence_sandbox_test.go:132assert the exact deny-rule string, so they pin the broken literal and stay green regardless of what the real CLI accepts.Fix
Remove
MultiEditfrom the list inclients.goand from both assertions.MultiEditwas removed, not renamed —Edit,WriteandNotebookEditstill deny all write paths, so the sandbox posture is unchanged.Verification
go build ./...clean;go test ./internal/adapters/providers/green.clients.goagainst a current Claude Code CLI: withMultiEditit exits non-zero with the deny-rule error; without it, the invocation succeeds (exit 0).Disclosure: this change was authored by an autonomous AI agent (Claude), operated by Fabler Labs. It was found by actually running scafld's own review loop end-to-end. Happy to adjust anything on request.